Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(files_sharing): Adjust design of account filter for file list #46857

Merged
merged 3 commits into from
Jul 30, 2024

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Jul 29, 2024

Summary

Fix the design of the file list filter:
image

Checklist

@susnux susnux added bug 3. to review Waiting for reviews labels Jul 29, 2024
@susnux susnux requested review from jancborchardt, nfebe and Pytal July 29, 2024 16:58
@susnux susnux requested a review from skjnldsv as a code owner July 29, 2024 16:58
@susnux susnux changed the title Fix/file list filter fix(files_sharing): Adjust design of account filter for file list Jul 29, 2024
Copy link
Member

@AndyScherzinger AndyScherzinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐘

@susnux susnux added this to the Nextcloud 30 milestone Jul 29, 2024
Copy link
Member

@Pytal Pytal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In updateAvailableAccounts I think sharees need to be filtered by type as a group share is treated as if it were an account

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Can we rename "Accounts" to "People" which sounds friendlier and also fits with the unified search? :)

@susnux
Copy link
Contributor Author

susnux commented Jul 30, 2024

@Pytal and @jancborchardt both of your comments are now implemented, thank you!

image

@susnux susnux force-pushed the fix/file-list-filter branch from 36f29d0 to 5f42452 Compare July 30, 2024 10:30
@susnux susnux requested review from jancborchardt and Pytal July 30, 2024 10:30
@skjnldsv
Copy link
Member

skjnldsv commented Jul 30, 2024

Slight request to the design team.
I think this is a bit technical looking and not super clear we're displaying filters.
Couldn't we add a label before this set of buttons/menus ?

The select menu is a bit more straightforward, as we display a summary of the selection before, and the actions labels speak for themselves with a verb in it
image

Maybe something like that (very rough mockup, but you get the idea)
image


EDIT: or aligning it with the selection checkbox instead
image

@blizzz blizzz mentioned this pull request Jul 30, 2024
@susnux
Copy link
Contributor Author

susnux commented Jul 30, 2024

Slight request to the design team.

I agree but can we do this in a follow up as this one here is fixing a regression from beta 1?

Maybe something like that (very rough mockup, but you get the idea)

(Just a note:) I really like this, but would still let the chips flow to the left side under the "filter" as otherwise the space is very limited on smaller devices.

@skjnldsv
Copy link
Member

I agree but can we do this in a follow up as this one here is fixing a regression from beta 1?

Absolutely! Sorry, I thought I approved already!! 🙈

@susnux susnux dismissed jancborchardt’s stale review July 30, 2024 11:34

Renamed to "People"

@susnux susnux added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Jul 30, 2024
@susnux susnux force-pushed the fix/file-list-filter branch from 712aa59 to b88f3ab Compare July 30, 2024 11:34
@blizzz
Copy link
Member

blizzz commented Jul 30, 2024

don't push, i will merge and compile assets with #46870

mh, cannot.

will build locally and force-merge. test was all green.

@blizzz blizzz force-pushed the fix/file-list-filter branch from b88f3ab to aab8ab4 Compare July 30, 2024 14:41
Signed-off-by: Arthur Schiwon <[email protected]>
@blizzz blizzz merged commit 412661a into master Jul 30, 2024
5 of 33 checks passed
@blizzz blizzz deleted the fix/file-list-filter branch July 30, 2024 14:44
@jancborchardt
Copy link
Member

Google Drive has the filters listed without a label as well, so it could be fine. Also, the "Filters" text and icon adds even more noise to the file list.

@jancborchardt
Copy link
Member

For reference this is how Google Drive and Dropbox do the filters, no label: #43241

@blizzz
Copy link
Member

blizzz commented Jul 31, 2024

@jancborchardt are you intentionally replying to a merged pull request?

@skjnldsv
Copy link
Member

For reference this is how Google Drive and Dropbox do the filters, no label: #43241

I guess the left padding is what makes it look weird in my opinion :)
No Google nor Dropbox does it
image

@nfebe
Copy link
Contributor

nfebe commented Jul 31, 2024

I guess the left padding is what makes it look weird in my opinion :)
No Google nor Dropbox does it

  • I agree, the left padding is unnecessary plus the filters keyword should not be added at all users can figure that out.
  • The filters should probably come before the recommended section (and those should be filter too)

Screenshot from 2024-07-31 13-14-41

But these should be moved to a follow up ticket/issue since this is already merged and might be spam-ish.

@susnux
Copy link
Contributor Author

susnux commented Jul 31, 2024

The filters should probably come before the recommended section (and those should be filter too)

This was discussed before and it is the same as with rich workspaces:
The filters should be directly above the file list content. But agree please move discussion to separate ticket :)

@BloodyIron
Copy link

It's not entirely clear to me, but... is this going to be full-width search presentation? As in, full-width of the page, not just a pull-down menu with limited view space? It looks like yes, but I would like to have this clarified.

Also, is this going to be able to present much larger amounts of results than just 10, and expanding by 10, each time?

Don't play with my heart! I've wanted to go back multiple Search overhauls for years! D:

@AndyScherzinger
Copy link
Member

@BloodyIron currently it only allows for filtering within the current place you are in within files but yes, actual search is also being looked into but not there yet. it is "in-app" so it is within the files user interface itself. You can already try it out with the v30 beta releases -but again, currently it only implemented filtering since search is a more complex implementation.

@BloodyIron
Copy link

@BloodyIron currently it only allows for filtering within the current place you are in within files but yes, actual search is also being looked into but not there yet. it is "in-app" so it is within the files user interface itself. You can already try it out with the v30 beta releases -but again, currently it only implemented filtering since search is a more complex implementation.

In the example of the "Files" app... is it recursive including and below the current scope when triggered? Or is it Files-wide (ALL the files!?) or...?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Broken file filter UI Redesign broken on mobile
8 participants